Conversation
jason-raitz
commented
Feb 13, 2026
jason-raitz
commented
- co-authored with @danschmidt5189
- some logs were still not getting request_id logged
- we believe there an issue with when the two around_performs were called
- this commit should eliminate that problem
jason-raitz
requested review from
anarchivist,
awilfox,
danschmidt5189 and
davezuckerman
danschmidt5189
left a comment
danschmidt5189
left a comment
There was a problem hiding this comment.
Curious about the test failures.
awilfox
left a comment
awilfox
left a comment
There was a problem hiding this comment.
This is good, but I am a bit concerned that the mutation of arguments is done in a method. If we change how or when (or if) that method is called, suddenly the arguments will change too. I think it's better to do this in a dedicated callback, i.e. before_perform :set_request_id or something.
There was a problem hiding this comment.
looks good. one clarificatory question about your docs in comments, but otherwise r+.
edit: this might be related to @awilfox's concern, too.
I think you're right. I'll redo it. I think the reason some of the jobs are not getting the request_id right now is possibly because of multiple before_performs and around_performs making it a race condition? I'll try something different in a bit. |
anarchivist
left a comment
anarchivist
left a comment
There was a problem hiding this comment.
👏 i like the direction this took! r+.
danschmidt5189
left a comment
danschmidt5189
left a comment
There was a problem hiding this comment.
Looks good, I like how this turned out, very clean / Rails-y.
awilfox
left a comment
awilfox
left a comment
There was a problem hiding this comment.
Looks great. Thanks for cleaning it up so well! r+
- co-authored with @danschmidt5189 - some logs were still not getting request_id logged - we believe there an issue with when the two around_performs were called - this commit should eliminate that problem
- replaces the positional removal with a destructuring and reassignment.
jason-raitz
force-pushed
the
AP-513_log_tracing_part_deux
branch
from
68e4764 to
2ce1587
Compare
4 participants
